Skip to content

added historical module - #5

Open
Big-Gray wants to merge 2 commits into
asymworks:masterfrom
Big-Gray:historical
Open

added historical module#5
Big-Gray wants to merge 2 commits into
asymworks:masterfrom
Big-Gray:historical

Conversation

@Big-Gray

@Big-Gray Big-Gray commented Sep 8, 2023

Copy link
Copy Markdown

I'm not sure why but historical observation calls always need a "T00-0000" appended to the date string. I haven't finished writing tests yet.

Comment thread pyairnow/historical.py Outdated
self,
zipCode: str,
*,
date: Optional[Union[date_, datetime, str]] = None,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even though the API allows a call with no date and returns an empty set of observations, it makes more sense for a historical API to make this required and do our own check for presence in the code. This seems to work a bit differently than the forecast which defaults to today if the parameter isn't sent.

Comment thread pyairnow/historical.py Outdated
Comment on lines +23 to +30
'''Airnow needs T00-0000 appended to the date parameter for historical calls'''
if date and isinstance(date, str):
y, m, d = date.split('-')
params['date'] = date_(int(y), int(m), int(d)).isoformat() + "T00-0000"
elif date and isinstance(date, datetime):
params['date'] = date.date().isoformat() + "T00-0000"
elif date and isinstance(date, date_):
params['date'] = date.isoformat() + "T00-0000"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is actually an odd form of extended ISO 8601 timestamp where the T00 is the time with only hours (minutes and seconds may be omitted if they are zero) and -0000 is the UTC offset. I think a better way to do it is to get the argument into a datetime, ensure it has a time zone offset set, then use strftime to put it into the correct %Y-%m-%dT%H%z format instead of relying on isoformat and hard-coded strings.

I probably should have done something similar originally in the Forecast module, although interestingly the Forecast API endpoint does not like the T00-0000 so the EPA is using different parsers on the backend.

Comment thread pyairnow/historical.py
Comment on lines +28 to +39
'''create a timezone object with no utc offset'''
tz = timezone(timedelta())
params['date'] = datetime(int(y), int(m), int(d), tzinfo=tz).strftime("%Y-%m-%dT%H%z")
elif isinstance(date, datetime):
tz = timezone(timedelta())
date = date.replace(tzinfo=tz)
params['date'] = date.strftime("%Y-%m-%dT%H%z")
elif isinstance(date, date_):
tz=timezone(timedelta())
params['date'] = datetime(date.year, date.month, date.day, tzinfo=tz).strftime("%Y-%m-%dT%H%z")
else:
raise AirNowError("Bad date type. Date parameter is either date, datetime, or a string.")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asymworks I switched to using strftime to get the right date formatting as you suggested. Interestingly, when a datetime object is set to utc timezone, the offset becomes +0000 and not -0000. I tested this with some simple calls to the api and it works just fine either way.

@asymworks

Copy link
Copy Markdown
Owner

@Big-Gray CI checks are failing due to running into a broken dependency with Python 3.12. I fixed the workflow in the master branch - please rebase your branch to pull those updates in and CI should pass.

@dmcc

dmcc commented Sep 9, 2024

Copy link
Copy Markdown

I'd be interested in this feature. I'm curious if there are any updates? I might also have some time to help out and rebase it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants